-
Notifications
You must be signed in to change notification settings - Fork 9
Go version/port of image build #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Could you add a README that documents a bit on how to use this version of the image builder? I didn't see one included in the diff. |
Sure, I can do that, but the intension is that its a "drop in replacement", the cli should take the same option and the configuration file format is the same. |
|
If that's the case, could you just add a few lines on any of the Go specific stuff? If it's supposed to be the same, I don't think there's a need for a whole new document. |
Sure, I will add something |
Done, feel free to reach out if you have questions or think other things should be added. |
|
Thanks for adding that. On another note, is there a supported way for building the Go binary? I built it by |
Your invocation is a supported approach 😄 , but yes we can add a Makefile like |
|
I ran [INSTALLER]2025/10/06 10:16:28 Cleaning up temporary directory /tmp/image-build-820598980
2025/10/06 10:16:28 Error building layer: failed to build layer: failed to install package groups: failed to install package groups: failed to run command in container: while running runtime: exit status 1I also saw this in the output which may be useful for troubleshooting. [INSTALLER]2025/10/06 10:16:28 Running command in container: [dnf -y groupinstall Minimal Install Development Tools --installroot=/mnt/target --setopt=reposdir=/mnt/target/etc/yum.repos.d --setopt=_db_backend=sqlite]
Main config did not have a _db_backend attr. before setopt
Unable to detect release version (use '--releasever' to specify release version)
Main config did not have a _db_backend attr. before setopt
You have enabled checking of packages via GPG keys. This is a good thing.
However, you do not have any GPG public keys installed. You need to download
the keys for packages you wish to install and install them.
You can do that by running the command:
rpm --import public.gpg.key
Alternatively you can specify the url to the key you would like to use
for a repository in the 'gpgkey' option in a repository section and DNF
will install it for you.
For more information contact your distribution or package provider.
Problem repository: [dl.fedoraproject.org_pub_epel_8_Everything_x86_64_]I'm not sure if it matters, but I'm not running a RHEL-based distribution. |
Let me take a look, I mainly tried the ones in |
|
@davidallendj The problem is that If we do want to maintain these options moving forward we can of cause do that too. I will update the README.md for now ... |
|
I tried with your recommendations as well as a few in the |
| // Parse command-line arguments first to determine if we need rootless setup | ||
| args, err := arguments.ParseCommandLine() | ||
| if err != nil { | ||
| log.Fatalf("Error parsing arguments: %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a bit of a nit pick, but other OpenCHAMI Go repos use github.com/rs/zerolog/log for logging. I'm not exactly sure what are our goals going forward with logging, but I figured I'd at least mention it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PCS is using github.com/sirupsen/logrus 😄 Happy to adopt anything ...
davidallendj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I tested and it worked like the Python version. Since we're only adding new files here, it should be okay to go ahead and merge this.
|
I would like to get some consensus of whether we think this golang version is a viable/desirable replacement for the existing python version. Thanks to @davidallendj for taking it for a spin. Do others want to weight in? It would be good to make a decision/plan before two version diverge ( the in coming PR is part of my motivation for post this comment ) |
This continues the work I was doing at the dev hackathon. I now have a pretty functional image-builder version in go, it's able to build a bunch of the example images, it supports: